Skip to content

feat: add group-based deployment filtering#365

Open
pippaye wants to merge 2 commits intoserokell:masterfrom
pippaye:add-group-support
Open

feat: add group-based deployment filtering#365
pippaye wants to merge 2 commits intoserokell:masterfrom
pippaye:add-group-support

Conversation

@pippaye
Copy link
Copy Markdown

@pippaye pippaye commented Mar 2, 2026

Summary

Add group-based filtering for deploy targets to allow partial deployments by logical groups.

Changes

  • Add groups generic option (profile/node/deploy) with set-merge semantics.
  • Add CLI flag --groups <GROUPS>... and filter deployment selection.
  • Document usage and add an example under examples/groups.
  • Example added in examples/groups.

Behavior

  • Effective groups are merged as a set from profile > node > deploy.
  • When --groups is provided, only profiles with any overlapping group are selected.
  • Profiles without groups are excluded when filtering is active.

Copilot AI review requested due to automatic review settings March 2, 2026 01:10
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds group-based deployment filtering to deploy-rs, allowing users to tag deploy targets (at profile, node, or deploy level) with logical group labels and then use a --groups CLI flag to limit which profiles get deployed. Groups from all three levels are merged into a union set for each profile. The feature includes a new groups field in GenericSettings, CLI flag integration, JSON schema update, and documentation/examples.

Changes:

  • New groups field (BTreeSet<String>) added to GenericSettings with custom serde deserialization (string or array) and set-union merge semantics via the merge crate.
  • New --groups <GROUPS>... CLI flag that filters the deployment loop to skip profiles whose merged group set has no overlap with the requested groups.
  • Documentation updates (README, JSON schema, example flake + README) covering the new groups option and its usage.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/data.rs Adds groups: BTreeSet<String> to GenericSettings with a custom deserializer (StringOrVec) and a set-union merge strategy
src/lib.rs Adds groups: Option<Vec<String>> field to CmdOverrides struct
src/cli.rs Adds --groups CLI argument and applies filter logic inside run_deploy; adds early-return guard when no profiles match
interface.json Adds groups property (string or array of strings) to the generic_settings JSON schema definition
examples/groups/flake.nix New example flake demonstrating multi-level group assignment
examples/groups/README.md Usage documentation for the new group filtering example
README.md Documents the groups generic option and --groups CLI flag in usage and API sections

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"type": "array",
"items": {
"type": "string"
}
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The groups schema in interface.json allows arrays but does not specify "uniqueItems": true. Since groups are modeled as a set (using BTreeSet in Rust, with duplicates silently deduplicated during deserialization), declaring uniqueItems: true would make the schema's intent clearer and allow schema validators to flag redundant entries. Compare profilesOrder at line 68 which already uses this constraint for similar reasons.

Suggested change
}
},
"uniqueItems": true

Copilot uses AI. Check for mistakes.
Comment on lines +57 to 75
fn deserialize_groups<'de, D>(deserializer: D) -> Result<BTreeSet<String>, D::Error>
where
D: Deserializer<'de>,
{
let value = Option::<StringOrVec>::deserialize(deserializer)?;
Ok(match value {
None => BTreeSet::new(),
Some(StringOrVec::String(s)) => {
let mut set = BTreeSet::new();
set.insert(s);
set
}
Some(StringOrVec::Vec(v)) => v.into_iter().collect(),
})
}

fn merge_groups(left: &mut BTreeSet<String>, right: BTreeSet<String>) {
left.extend(right);
}
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The deserialize_groups function (handling both string and array inputs) and merge_groups function (set-union semantics) have no unit tests. The codebase has tests for similar data-processing functions (e.g., test_parse_flake in src/lib.rs, command builder tests in src/deploy.rs). Tests for these functions would verify:

  • Deserializing a string value (e.g. groups = "blue") creates a single-element set
  • Deserializing an array of strings creates the correct set
  • Deserializing an absent field creates an empty set
  • Two overlapping sets merge correctly (union, no duplicates)

Copilot uses AI. Check for mistakes.
}

if parts.is_empty() {
info!("No profiles matched selection.");
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When --groups is specified and all profiles are filtered out, the function returns Ok(()) with just an info! log message "No profiles matched selection." This means the process exits with code 0, even when the user may have explicitly targeted a profile via a flake fragment (e.g., myflake#mynode.myprofile --groups somegroup). A user who misspelled a group name or forgot to assign groups to profiles would see no deployment occur but no error either, which can silently mask deployment mistakes.

Consider returning an error (or at minimum a warn! or error! level message) when groups filtering is active and results in zero matched profiles, so users get a clear signal that something may have gone wrong.

Suggested change
info!("No profiles matched selection.");
if cmd_overrides.groups.is_some() {
error!("No profiles matched selection after applying group filters. This may indicate a misspelled group name or missing group assignments.");
} else {
info!("No profiles matched selection.");
}

Copilot uses AI. Check for mistakes.
sshOpts = [ "-p" "2121" ];

# Optional groups used for filtering deployments.
# Can be a string or a list of strings; values from profile > node > deploy are merged as a set (deduplicated).
Copy link

Copilot AI Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The README documentation states groups values are "merged as a set (deduplicated)" from "profile > node > deploy", but the > notation is used throughout the README (line 185) to denote override priority (where a profile's value takes precedence over the node's value). For groups specifically, ALL values from all three levels are unioned together — a profile with no groups still inherits groups from node and deploy levels. This union semantics is different from the override semantics of all other generic settings and is not clearly distinguished in the documentation. The comment should explicitly state that unlike other settings, groups from all levels are combined (not overridden), so users don't expect only the most specific level's groups to be used.

Suggested change
# Can be a string or a list of strings; values from profile > node > deploy are merged as a set (deduplicated).
# Can be a string or a list of strings; unlike other settings that follow
# profile > node > deploy override semantics, group values from all three
# levels are combined (unioned and deduplicated). A profile with no groups
# still inherits groups defined at the node and deploy levels.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants